Adopt IBKR runtime target JSON wiring - #52
Conversation
…i-deps # Conflicts: # requirements.txt
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 317b22b426
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw_runtime_target = os.environ.get("RUNTIME_TARGET_JSON", "").strip() | ||
| if not raw_runtime_target: | ||
| raise SystemExit("RUNTIME_TARGET_JSON is required") | ||
| runtime_target = json.loads(raw_runtime_target) | ||
| profile = str(runtime_target.get("strategy_profile") or "").strip().lower() | ||
| if not profile: | ||
| raise SystemExit("STRATEGY_PROFILE is required") | ||
| raise SystemExit("RUNTIME_TARGET_JSON.strategy_profile is required") |
There was a problem hiding this comment.
The workflow now forwards RUNTIME_TARGET_JSON to Cloud Run, but this step only validates strategy_profile and never checks required fields like platform_id and dry_run_only (or execution_mode consistency). The runtime loader now enforces those fields via resolve_runtime_target_from_env, so an incomplete JSON (for example, one containing only strategy_profile) will pass CI here and then fail service startup after env sync, causing a broken revision in production.
Useful? React with 👍 / 👎.
| runtime_target = resolve_runtime_target_from_env(env=os.environ, expected_platform_id=IBKR_PLATFORM) | ||
| strategy_definition = resolve_strategy_definition( | ||
| os.getenv("STRATEGY_PROFILE"), | ||
| runtime_target.strategy_profile, | ||
| platform_id=IBKR_PLATFORM, |
There was a problem hiding this comment.
This change makes RUNTIME_TARGET_JSON mandatory and uses it to choose the strategy profile, but it does not reconcile runtime_target.dry_run_only with the actual execution setting (still read from IBKR_DRY_RUN_ONLY). That creates a split-brain configuration where the target says paper/dry-run while execution can still run live if the env var is unset or different, which is a high-risk behavior mismatch introduced by the new wiring.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109d4f2061
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return self.reporting_builder( | ||
| runtime_assembly=runtime_assembly, | ||
| strategy_domain=self.strategy_domain, |
There was a problem hiding this comment.
After this refactor, build_reporting_adapters() only passes runtime_assembly and no longer forwards extra_context_fields to build_runtime_reporting_adapters. The reporting adapter still builds summary.account_ids from self.extra_context_fields (application/runtime_reporting_adapters.py), so production reports generated via this composer will now emit empty account_ids even when account IDs are configured, reducing execution traceability and report accuracy.
Useful? React with 👍 / 👎.
1 participant
Summary
RUNTIME_TARGET_JSONfor IBKR runtime settings and env syncTests
PYTHONPATH=.:../QuantPlatformKit/src:../UsEquityStrategies/src .venv/bin/python -m pytest tests/test_runtime_config_support.py tests/test_notifications.py -qbash tests/test_sync_cloud_run_env_workflow.sh